Use kprobe for unreliable recvmsg return probe #1095
Conversation
grcevski
requested review from
mariomac,
marctc and
rafaelroquetto
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1095 +/- ##
==========================================
- Coverage 73.26% 70.65% -2.61%
==========================================
Files 172 172
Lines 13066 13073 +7
==========================================
- Hits 9573 9237 -336
- Misses 2944 3272 +328
- Partials 549 564 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -502,6 +495,32 @@ int BPF_KRETPROBE(kretprobe_tcp_recvmsg, int copied_len) { | |||
| return 0; | |||
| } | |||
|
|
|||
| SEC("kprobe/tcp_cleanup_rbuf") | |||
| int BPF_KPROBE(kprobe_tcp_cleanup_rbuf, struct sock *sk, int copied) { | |||
| u64 id = bpf_get_current_pid_tgid(); | |||
There was a problem hiding this comment.
just a nitpick:
| u64 id = bpf_get_current_pid_tgid(); | |
| const u64 id = bpf_get_current_pid_tgid(); |
If I could make a recommendation, I'd suggest we start using const by default on our codebase moving forward, and dropping it only when required. There's a rationale about const-correctness here - it's mostly C++ oriented but it also applies to C (to an extent, in particular to preventing logic errors, improving clarity and optimisations, although I don't know how the latter goes for eBPF). Another interesting example is Rust. It makes everything const by default (and provides the mut keyword to explicitly tag variables that are not const)
There are number of reasons why a kretprobe may not fire and we've recently had issues on older kernels where we don't see some return probes. This PR adds code to run the same logic we have on the kretprobe for tcp_recvmsg on a kprobe for tcp_cleanup_rbuf. The retprobe is still there in case we encounter custom kernel builds (like we've seen before with WSL2) where the tcp_cleanup_rbuf might be inlined.
Tested with 5.10 and 6.5.